sql: improve physical planning of window functions #85355
Conversation
It doesn't seem to be used much. Release note: None
|
This change is |
yuzefovich
force-pushed
the
wf
branch
6 times, most recently
from
58a4065 to
732b206
Compare
There was a problem hiding this comment.
Nice refactoring!
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @rytaft and @yuzefovich)
pkg/sql/distsql_physical_planner.go line 3760 at r2 (raw file):
// Check that all window functions have the same PARTITION BY and ORDER BY // clauses.
[nit] Maybe add a comment mentioning that the optbuilder ensures that all window functions in the node have the same PARTITION BY and ORDER BY clauses.
This commit improves the physical planning of window functions in several ways. First, the optimizer is updated so that all window functions with a PARTITION BY clause are constructed first followed by the remaining window functions without PARTITION BY. This is needed by the execution which can only evaluate functions with PARTITION BY in the distributed fashion - as a result of this change, we are now more likely to get partial distributed execution (previously things depended on the order in which window functions were mentioned in the query). Second, the physical planner now thinks that we "should distribute" the plan if it finds at least one window function with PARTITION BY clause. Previously, we didn't make any recommendation about the distribution based on the presence of the window functions (i.e. we relied on the rest of the plan to do so), but they can be quite computation-intensive, so whenever we can distribute the execution, we should do so. Additionally, this commit removes some of the code in the physical planner which tries to find window functions with the same PARTITION BY and ORDER BY clauses - that code has been redundant for long time given that the optimizer does that too. Release note: None
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/sql/distsql_physical_planner.go line 3760 at r2 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
[nit] Maybe add a comment mentioning that the optbuilder ensures that all window functions in the node have the same
PARTITION BYandORDER BYclauses.
Done.
|
Build succeeded: |
sql: remove shouldNotDistribute recommendation
It doesn't seem to be used much.
Release note: None
sql: improve physical planning of window functions
This commit improves the physical planning of window functions in
several ways.
First, the optimizer is updated so that all window functions with a
PARTITION BY clause are constructed first followed by the remaining
window functions without PARTITION BY. This is needed by the execution
which can only evaluate functions with PARTITION BY in the distributed
fashion - as a result of this change, we are now more likely to get
partial distributed execution (previously things depended on the order
in which window functions were mentioned in the query).
Second, the physical planner now thinks that we "should distribute" the
plan if it finds at least one window function with PARTITION BY clause.
Previously, we didn't make any recommendation about the distribution
based on the presence of the window functions (i.e. we relied on the
rest of the plan to do so), but they can be quite computation-intensive,
so whenever we can distribute the execution, we should do so.
Additionally, this commit removes some of the code in the physical
planner which tries to find window functions with the same PARTITION BY
and ORDER BY clauses - that code has been redundant for long time given
that the optimizer does that too.
Release note: None